%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Math call site\ne.g. WWMath::Sinf] --> B{IS_VS6_BUILD?}
    B -- Yes --> C[x87 inline asm\nfsin/fcos/fsqrt]
    B -- No --> D{USE_DETERMINISTIC_MATH?}
    D -- No --> E[CRT system math\nsinf/cosf/sqrtf]
    D -- Yes --> F[GameMath fdlibm\ngm_sinf/gm_cosf]

    G[BaseDefines.h] --> H{RETAIL_COMPATIBLE_CRC == 1?\ndefault: yes}
    H -- Yes --> I[undef USE_DETERMINISTIC_MATH\nsilent CRT fallback]
    H -- No --> J{__has_include gmath.h?}
    J -- No --> I
    J -- Yes --> K[USE_DETERMINISTIC_MATH active]

    L[cmake/gamemath.cmake\nFetchContent GameMath] --> M[include_directories\ngmath.h available globally]
    M --> J

    style C fill:#FFA500
    style E fill:#6699FF
    style F fill:#66BB66
    style I fill:#AAAAAA

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Math call site\ne.g. WWMath::Sinf] --> B{IS_VS6_BUILD?}
    B -- Yes --> C[x87 inline asm\nfsin/fcos/fsqrt]
    B -- No --> D{USE_DETERMINISTIC_MATH?}
    D -- No --> E[CRT system math\nsinf/cosf/sqrtf]
    D -- Yes --> F[GameMath fdlibm\ngm_sinf/gm_cosf]

    G[BaseDefines.h] --> H{RETAIL_COMPATIBLE_CRC == 1?\ndefault: yes}
    H -- Yes --> I[undef USE_DETERMINISTIC_MATH\nsilent CRT fallback]
    H -- No --> J{__has_include gmath.h?}
    J -- No --> I
    J -- Yes --> K[USE_DETERMINISTIC_MATH active]

    L[cmake/gamemath.cmake\nFetchContent GameMath] --> M[include_directories\ngmath.h available globally]
    M --> J

    style C fill:#FFA500
    style E fill:#6699FF
    style F fill:#66BB66
    style I fill:#AAAAAA

_{Reviews (9): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

I’m testing this on a separate branch:
https://github.com/Okladnoj/GeneralsGameCode/okji/test/deterministic-math-v2

I slightly adjusted the CI there so I can run Win32 and get access to the game resources.

You did not review the changes you made with AI. It has issues that you should fix before asking it to be reviewed.

This change does too many things. It is better to first consolidate trig and wwmath and maybe other sources of math, before going into gamemath territory.

This change does too many things. It is better to first consolidate trig and wwmath and maybe other sources of math, before going into gamemath territory.

@xezon Hey! I understand your point, but the reason I didn't fully consolidate trig and wwmath in this PR is exactly to avoid doing too many things at once.

As we saw in PR #2602, fully removing trig.h and replacing it with WWMath across the codebase touches over 120 files. Mixing a massive 120+ file architectural refactoring with a core feature addition (GameMath) made the previous PR extremely difficult to review and broke compilation for some standalone utilities, because trig.h is used outside of just game math.

That's exactly why I chose this "routing" approach for this PR. By keeping the trig.h interface intact and just routing its internal implementation to WWMath, we achieve the deterministic math goals with a much smaller and safer footprint.

Perhaps the best option would be to test this PR first, and if everything is fine — merge it. And only after that, we can focus on a second PR dedicated purely to the architectural cleanup (removing trig.h across all 120+ files)?

Hi @xezon! I have addressed all your review feedback points and updated the PR.

CI Status:
The CI is completely green. I ran the benchmarks on both Win32 and VC6 with the latest changes, and the CRC results perfectly match our previous deterministic baselines (76B53840 for deterministic, E8B6385A for native).

To save you from hunting through all the comment threads, here is a consolidated list of the answers and solutions to your review points:

• Function_Name convention / Naming inconsistencies
  Fixed. Renamed all math wrappers to use the _Origin and _Trig convention. The _Trig suffix also cleanly resolves conflicts with legacy EA names (e.g., ACos_Trig vs Acos).
• Move #define next to #include gmath
  Fixed.
• Redundant VC6 guard
  Fixed — removed the outer #if !(defined(_MSC_VER)...) guard, kept only __has_include. VC6 doesn't support __has_include, so the block is naturally skipped.
• "origin" terminology
  "Origin" means the original EA code called bare CRT functions (sqrt, acos, sinf...). The suffix explicitly marks which exact CRT function was used originally. These are not just type variants — they are different precision math paths.
• Missing gm math variants / CeilfOrigin identical to Ceil
Ceil(float) and Floor(float) are original EA code used only in rendering (visrasterizer.cpp). Determinism isn't needed there. However, CeilfOrigin(float) is a game logic wrapper that routes to gm_ceilf. Therefore, they are not identical.
• C++ overloads instead of f suffix
  Overloads are dangerous here. GameMath only provides float functions (the double version always narrows). With overloads, the compiler silently picks the version by argument type and could inadvertently change the precision path. Explicit names protect against this.
• No @fix prefix in CMake / What is ODR? / Line breaks / iters typo
  Fixed.
• Benchmark in GameLogic::update()
  Moved the auto-benchmark out of the replay loop. It is now a simple compile-time flag. (Did not prepare an ImGui stub since ImGui does not exist in the project).
• VS6 exclusion necessary in CMake?
  Yes, it is necessary. VC6 doesn't support <stdint.h> and long long required by GameMath. Removing the exclusion will break the build.
• Sqrt(double) intentional in BaseType.h?
  Yes, intentional. Coord3D::length() is used in game logic and participates in CRC — it must be strictly deterministic.

Hi @xezon! Thanks for the detailed review. I agree with some of your points regarding code cleanliness (I will remove the Ceil/Floor wrappers for the renderer).

However, there are a couple of critical architectural points concerning the preservation of old replays (suffixes) and determinism (Trig.cpp) that I want to clarify before pushing changes.

1. C++ Overloads vs Explicit types (why suffixes are needed)

I want to explain why I had to come to an explicit separation of functions via suffixes instead of using C++ overloads. This is tied to the necessity of preserving 100% backwards compatibility for old builds (VC6 Retail Compatibility).

I introduced 3 types of functions because they reflect 3 completely different mathematical paths (math paths) in the original EA engine. Our codebase serves three build modes at once (VC6, Win32, and Deterministic), and if we don't strictly fix the paths, we will lose Retail compatibility on old compilers:

1. Without suffix (WWMath::Cos): This is the original Westwood Math implementation. In the original game on VC6/Win32, it compiles into inline x87 asm (fcos).
2. With _Trig suffix (WWMath::Cos_Trig): This is a replacement for the global Cos() function from Trig.cpp. In the original game on VC6, it called the CRT function cosf() (not fcos!). The difference in the lowest bits between fcos and cosf() is critical: if I merge them into a single function without a suffix, the retail build will start calling fcos instead of cosf(), and the original logic will break.
3. With _Origin and f_Origin suffixes (ACos_Origin vs ACosf_Origin): These replace direct system calls to acos(double) and acosf(float) in GameLogic. The deterministic library GameMath provides only float versions. My double version is forced to do a narrowing cast: (double)gm_acosf((float)x).
   The original EA code often passed variables of type float into system functions expecting double (e.g., acos()), relying on automatic type promotion by the compiler.
   If I switch to C++ overloads (just ACos), then when passing a float, the compiler will automatically pick the float overload. This will change the original math path (instead of calling the double version with narrowing, it will call the pure float version).

Explicit suffixes strictly lock the original execution path. They guarantee that the exact function intended in the original game is called, avoiding unpredictable compiler behavior during overload resolution.

Examples (The mechanics of overload conflicts)

Here is, with examples, how the overload mechanism breaks the original branches when compiling under VC6:

Example A: Conflicting identical signatures (_Trig)
In the original game, we had two different math paths that took the exact same type (float), but executed different instructions:

1. The original WWMath::Cos(float) → compiled into fcos (inline asm).
2. The original Trig::Cos(float) → compiled into cosf() (CRT).

C++ overloads only work with different argument types. How is the compiler supposed to know which of the two Cos(1.0f) calls should go to assembler, and which should go to the system CRT, if their signatures are absolutely identical? It can't.
If we remove _Trig and leave only WWMath::Cos(float), then in the VC6 build, all code from the former Trig.cpp will start invoking the fcos assembler instead of the original cosf(). The math is broken.

Example B: Path substitution via typing (_Origin)
On the calling code side in GameLogic, EA often wrote like this:

float myVal = 0.5f;
float result = acos(myVal); // In the original, this is a call to <math.h> double acos(double)

Since acos in C accepted a double, the compiler did an implicit cast: float -> double -> acos(double) -> float.

What happens if we introduce the overloads WWMath::ACos(float) and WWMath::ACos(double)?
The call to WWMath::ACos(myVal) will see the float type. The C++ overload mechanism will directly call the float overload, completely ignoring the original path with promotion to double. The VC6 logic is broken! The explicit suffix ACos_Origin(double) takes away the compiler's right to choose and strictly forces the original math path.

2. Sqrt(double) in BaseType.h:391

And (Real)sqrt( x*x + y*y + z*z ); was calling double sqrt(double) ?

Yes, in the original game it fell back to the system CRT double sqrt(double). But the problem is that Coord3D::length() is actively used in game logic (it participates in physics and CRC calculations). If I leave the system double, we will have discrepancies between Mac, Win32, and VC6. I have to forcibly cast it to deterministic float (at the cost of precision loss) to guarantee cross-platform sync.

3. "Trampolines" in Trig.cpp

What is the point of moving the function body to WWMath... No trampoline to WWMath.

The fact is that I was acting exactly according to your original task from the previous PR (#2602).
You wrote then: "Generally it is a bad sign if simplifying code would break something. If so, it needs to be fixed", and asked me to physically delete the old Trig.cpp files, migrating everything to WWMath.

I did exactly that. But stephanmeesters discovered that completely deleting trig.h breaks the VC6 / Win32 compilation (over 120 files are affected due to implicit includes).
To save the VC6 build, I had to restore the old Trig.h interface.

But I moved the implementation itself to wwmath.h to fulfill your requirement for math consolidation. If I write #if USE_DETERMINISTIC_MATH directly inside Trig.cpp, I will have to do it twice (since there are two Trig.cpp files in the engine — in Generals and GeneralsMD).
The trampoline is a transitional compromise that allowed us to not break the VC6 build and to gather the deterministic logic strictly in one place, as we planned. In the second phase, when there is already a working system with deterministic math in the main branch, we can start looking for the best way to delete trig.h and fully rely on wwmath.

4. Duplicates (Ceil / Floor)

Regarding Ceil and Floor — here I completely agree with you.
Since these functions (along with their original EA versions) are used exclusively in rendering (e.g., in visrasterizer.cpp) and do not participate in CRC calculations for network play, wrapping them in WWMath makes no sense.
I will completely remove these wrappers from wwmath.h and write direct calls to std::ceil / std::floor right at their call sites in the render code.

Hi @xezon! Thanks for the detailed review. I agree with some of your points regarding code cleanliness (I will remove the Ceil/Floor wrappers for the renderer).

However, there are a couple of critical architectural points concerning the preservation of old replays (suffixes) and determinism (Trig.cpp) that I want to clarify before pushing changes.

It is a bit tough to fight through this much AI generated text. Please push the last state of the code and then I can take a look at it in Visual Studio and try to polish it up if it needs polishing. I expect this is faster than chatting about where to go with this. Generally, try to not trust the AI generated code too much. It generates code that is for machines, not humans.

It is a bit tough to fight through this much AI generated text. Please push the last state of the code and then I can take a look at it in Visual Studio and try to polish it up if it needs polishing. I expect this is faster than chatting about where to go with this. Generally, try to not trust the AI generated code too much. It generates code that is for machines, not humans.

I wrote every point personally — I only asked AI to format it properly, fix spelling, and translate it into English, exactly like I’m asking now, because my English is not very strong.

I personally worked through every point of that long text, so it would be better to read it carefully and understand the reasoning behind it — there is nothing unnecessary there.

The main point is that suffixes like _Trig and _Origin are physically necessary for us, because overloading cannot handle this task properly.

In the original project, before deterministic math was introduced, there were places with mixed math inside the game logic that affects the CRC. When USE_DETERMINISTIC_MATH is disabled, we need to support the old CRC calculation system, which means we need simultaneous _Trig and _Origin implementations.

If we could simply remove USE_DETERMINISTIC_MATH from the project, there would not be such a large-scale transformation and interweaving of math functions. But in the old mode, we support not only Win32, but also VC6 with its own assembly functions.

Hi @xezon! Thanks for the detailed review. I agree with some of your points regarding code cleanliness (I will remove the Ceil/Floor wrappers for the renderer).

@xezon
In short, I don’t think it can be explained much shorter or simpler than in that message.

The project’s math was not always written with a clean and transparent architecture — or at least not all parts of it were. Maybe this was even done intentionally to make it harder to reverse-engineer the CRC logic.

At the moment, all workflows build successfully, and all replays also play successfully both with deterministic math enabled and disabled.

Above, I sent a screenshot of your job, plus one additional replay run that I configured specifically to verify Win32.

Ok fair comments. I was under the impression I was chatting with AI generated text because of all the polished formatting. Can you push the latest state to the branch that you have now? I would like to take a look at it in Visual Studio next.

Btw, Replay Check is currently broken. We need to wait until after that is fixed.

Ok fair comments. I was under the impression I was chatting with AI generated text because of all the polished formatting. Can you push the latest state to the branch that you have now? I would like to take a look at it in Visual Studio next.

Btw, Replay Check is currently broken. We need to wait until after that is fixed.

The branch is already up to date — I haven't made any changes since the last push, I was waiting for your feedback. Feel free to take the current branch and work on it in VS. If you need my help — push your changes and I'll pick up from there.

Regarding the broken Replay Check — the CI runner has no way to obtain the game data. I solved this by extracting a minimal set of files from the Steam distribution (no textures, audio, or GUI — just enough for replay verification), uploaded them as a release to a private repository (Okladnoj/generals-gamedata), and connected it to the workflow via a PAT secret (GAMEDATA_PAT). The CI downloads the data using gh release download, verifies SHA256, and uses it for replay check. You can see the configuration on the test branch: okji/test/deterministic-math-v2 — file .github/workflows/check-replays.yml. Feel free to adopt this approach — or give me access to your organization, and I'll create a similar private repo with the data and wire it up to your CI.

The branch is already up to date

The last push in from 08 May

Polished and pushed to the wip-deterministic-math-v3 branch a bit more.

Caball tested it against 2500 Replays and it passed.

@Okladnoj please test the determinism with 2 game clients.

Polished and pushed to the wip-deterministic-math-v3 branch a bit more.

Caball tested it against 2500 Replays and it passed.

@Okladnoj please test the determinism with 2 game clients.

I ported the deterministic math version 3 to the Mac port project.
https://github.com/xezon/CnC_Generals_Zero_Hour/tree/xezon/wip-deterministic-math-v3
↓
https://github.com/Okladnoj/GeneralsGameCode/tree/okji/test/deterministic-math-v3
↓
https://github.com/OKJID/GameClient/tree/okji/feat/macos-port

And I played a game on the Twilight Forest map (duration about 30-40 minutes) and recorded a replay.

Since I don't have a proper Windows setup that can run the game, I used CI to test the Mac replay on a Windows machine. The replay played successfully on both the old vc6 compiler build and the new win32 compiler build without any desyncs.

Can any of the Windows developers build the game client with v3 math and play a multiplayer game with me (I will be on Mac OS)?

Ok that is very promising indeed.

I've added your changes to my own fork running one windows machine and one mac machine. These changes are amazing, and I ran into 3 additional mismatch issues, but now have it working for me with Mac M4 vs Windows on a local network game. These issues/fixes have been working for me, though still need more heavy testing:

1. The x87-vs-SSE2/NEON precision model. Single precision works but double doesn't- fix: Keep _PC_24 and make the simulation single-precision only (swept ~87 WWMath::Atan2/Sqrt/Sin/Cos/Pow(double) calls to the f-variants; killed stray double locals/literals). The clean end-state would be a 64-bit Windows build (no x87 at all), but the codebase is currently 32-bit-locked (x86 inline asm + a hard mingw guard).
2. (int)NaN / (int)Inf is architecture-divergent UB. Casting NaN/Inf to int — directly, or via REAL_TO_INT_CEIL/FLOOR (which is lroundf on arm64 but x87 fistp on Windows) — gives different results per arch (arm64 → 0, x86 → INT_MIN/garbage). Fix: found & guarded ~14 of these.
3. wchar_t wire-format width. wchar_t is 4 bytes on macOS, 2 on Windows; any struct/buffer serialized as raw wchar_t corrupts cross-platform. Fixed in LANAPI, QuotedPrintable, and in-game chat (16-bit UTF-16 LE everywhere).

@Okladnoj The vc6 string format fix is merged. Do you need help here splitting the math refactors from changes or can you do it?

@Okladnoj The vc6 string format fix is merged. Do you need help here splitting the math refactors from changes or can you do it?

@xezon
During testing and alignment between the Windows build and the native Mac build, several determinism violations were found.

I added a few additional overloads here:
https://github.com/Okladnoj/GeneralsGameCode/blob/okji/test/deterministic-math-v3/Core/Libraries/Source/WWVegas/WWMath/wwmath.h#L143-L159
to avoid inconsistencies between Mac and Windows.

All math non-determinism was confirmed by these tests:
https://github.com/Okladnoj/GeneralsGameCode/blob/okji/test/deterministic-math-v3/Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp#L38-L302
by comparing the Mac and Windows test results.

At the moment, the game runs without desyncs as long as rockets are not fired.

The desync happens for a different reason: the asynchronous behavior of the render engine on Mac breaks the creation of “unit bones”, which are used to calculate rocket spawn positions. I am still working on this here:
https://github.com/OKJID/GameClient/tree/okji/feat/macos-port-lan/
because after fixing projectile spawning, I may still find some additional sources of non-determinism.

For now, both branches are very dirty — there is a lot of detailed logging added there.

I'm impressed that 32bit builds using x87 fpu are able to be matched with other platforms at all to be honest, I never expected any of the game maths stuff to generate matching results to other none x86 platforms without fully using SSE/SSE2.

I added a few additional overloads here: https://github.com/Okladnoj/GeneralsGameCode/blob/okji/test/deterministic-math-v3/Core/Libraries/Source/WWVegas/WWMath/wwmath.h#L143-L159 to avoid inconsistencies between Mac and Windows.

This looks like it makes sense. I suggest to sort them between between their respective buddies.

Hi @xezon !

check please xezon#10

The GameLogic change needs to be a separate pull request. Rebuild.ps1 does not look like it belongs into the repository. Default value of RETAIL_COMPATIBLE_CRC should not be changed. Math float function overloads can be sorted better to their buddy functions.

We need to take all the math changes and split them into smaller commits, 2 or 3 final, to decouple refactorings from adding new math variants. Can you do it? Otherwise I can when I eventually get time for it.

The GameLogic change needs to be a separate pull request. Rebuild.ps1 does not look like it belongs into the repository. Default value of RETAIL_COMPATIBLE_CRC should not be changed. Math float function overloads can be sorted better to their buddy functions.

We need to take all the math changes and split them into smaller commits, 2 or 3 final, to decouple refactorings from adding new math variants. Can you do it? Otherwise I can when I eventually get time for it.

Like now?:
xezon#11
xezon#10

Like now?

Unless there is more to fix first.

Unless there is more to fix first.

Everything is done from my side. Further work will only make sense once the deterministic math is ported to the GOD-team project. For one, the game engine runs at 60 Hz there instead of 30 Hz, and the codebase is also slightly different.

Integration of xezon's refactoring + fixes from PR xezon#10

Following advice from mirelle (community patch developer/contributor — Discord), I've consolidated everything into this single PR for easier review.

What changed

The branch now includes:

1. @xezon's 61-commit refactoring from xezon/wip-deterministic-math-v3 — restructured the USE_DETERMINISTIC_MATH dependency on RETAIL_COMPATIBLE_CRC, cleaned up function naming and organization in wwmath.h.

2. 2 additional fixes from xezon/CnC_Generals_Zero_Hour#10, discovered during real cross-platform LAN testing (native macOS ARM64 vs Windows x86):

   • wwmath.h: Added float overloads for Sin, Cos, Tan, Asin, Acos, Atan, Atan2, Sqrt, Fabs, etc. Without these, the compiler implicitly promotes float arguments to double, causing 1 ULP divergence between ARM64 (NEON, 64-bit rounding) and x86 (x87 FPU, 80-bit intermediate rounding).
   • BitFlags.h: Changed unsigned long to unsigned __int64 for VC6 compatibility.

3. Merge with latest origin/main — resolved 4 trivial conflicts:

   • BaseType.h: new ICoord2D methods (add, sub, operator+=, lengthSqr) — accepted, Sqrt preserved
   • wwmath.h: asm block formatting (braces) — accepted from main
   • InGameUI.cpp (×2): new isInForceAttackMode() angle snapping logic — accepted, Round → Roundf for determinism
   • LookAtXlat.cpp: deleted in main — accepted deletion

Testing results

I performed detailed testing of a real native game without any wrappers, Wine, or Parallels — Mac vs Windows directly:

• 12,000+ frames of LAN multiplayer (USA vs USA, Akas Magic map) with zero CRC desyncs.
• Several root causes of desync were found and fixed in the process (documented in xezon/CnC_Generals_Zero_Hour#10).

@xezon @OmniBlade @bobtista @Caball009